Conversation
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
Tried giving this a run-through in the browser. We're getting close, but still a few rough edges. Most of my comments aren't really in scope, but just a record of what I'd observed. For what this is aiming to accomplish, it's a good step forward 👍
| front_image_url: params[:front_image_url], | ||
| back_image_url: params[:back_image_url], | ||
| selfie_image_url: params[:selfie_image_url], | ||
| liveness_checking_enabled: params[:liveness_checking_enabled], |
There was a problem hiding this comment.
One thing I noticed in trying to test this in the browser is that we don't actually send an explicit parameter for this from the client. We could, or it also seems we could also reasonably derive this from the presence of a selfie_image_url. If we did that, not sure if it makes sense to consider here, or from within identity-idp-functions. I'd lean toward identity-idp-functions to have fewer things to wrangle.
There was a problem hiding this comment.
Great catch!
Since this this derives from DocAuthBaseStep we can just derive this from the server:
identity-idp/app/services/idv/steps/doc_auth_base_step.rb
Lines 174 to 176 in 97fe583
so even if the client sends a URL when it shouldn't, it wouldn't change the behavior?
| trace_id: amzn_trace_id, | ||
| } | ||
| ).run do |doc_auth_result| | ||
| document_capture_session.store_proofing_pii_from_doc(doc_auth_result) |
There was a problem hiding this comment.
In trying to test this in a real browser, I notice that we're not receiving the actual (fake) PII with doc_auth_result. Should we be?
I think we may still need some revisions to our logic to determine document status verification as "done" since it seems the actual result is being stored as proofing_job_result.pii[:document_result].
There was a problem hiding this comment.
Great catch, I think that means we need to be need to be grabbing :document_result in the capture block locally, the controller already grabs that key: https://github.com/18F/identity-idp/blob/master/app/controllers/lambda_callback/document_proof_result_controller.rb
and I think I need to update the controller to actually contain the correct PII
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
…-idp into margolis-async-doc-auth-job
2 participants
Needs a few tweaks to the gem to be merged first: 18F/identity-idp-functions#42
Once this is merged, we should be able to test the upload to S3 flow in lower envs, even with our fake YAML data instead of images